Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

feat($http): Added method to abort a pending request. (2nd attempt) #1836

Closed
wants to merge 1 commit into from

Conversation

dbinit
Copy link
Contributor

@dbinit dbinit commented Jan 18, 2013

Fixes issue #1159. Was pull request #1623.

Addressed issues brought up by Miško:

  1. After calling .abort(), the promise is guaranteed to be resolved with a rejection (unless it has already been resolved).
  2. The .abort() method now returns a boolean:
    • True - if successfully aborted (or already aborted).
    • False - if abort failed because the promise is already resolved.
  3. Updated $httpBackend mock to support aborting.

@rickkln
Copy link

rickkln commented Jan 23, 2013

I am currently trying to use this solution and at first it seemed to be working but now I'm getting a strange problem that I can't seem to solve.

I am watching three drop downs on a dashboard: Year, Month and Day. Each time the value in the drop down changes I want to query for the new dashboard for that new date. However sometimes the user changes the month and then the day in quick succession. I therefore want to abort the query for the first date and only send the query for the final date.

I am currently trying to implement the solution like this:

 $scope.$watch('dash_year', function() {
    if($scope.current_request != undefined) {
      $scope.current_request.abort();
    }
    $scope.dashboard_loading = true;
    getDashboard();
  });
$scope.$watch('dash_month', function() {
    if($scope.current_request != undefined) {
      console.log($scope.current_request);
      $scope.current_request.abort();
    }
    $scope.dashboard_loading = true;
    getDashboard();
  });
$scope.$watch('dash_day', function() {
    if($scope.current_request != undefined) {
      $scope.current_request.abort();
    }
    $scope.dashboard_loading = true;
    getDashboard();
  });

If I make some quick changes to the month I get this in the console:
First the console.log($scope.current_request); to see if it is trying to abort an actual request, that returns this:
Screen Shot 2013-01-23 at 1 17 09 PM
So yes it seems $scope.current_request has the correct value.

But then the console says this:
Screen Shot 2013-01-23 at 1 18 37 PM

For me it does this every time I try to abort a request, always "$digest already in progress".

What would be the correct way to implement this abort request solution in conjunction with a $watch?

@fredrikbonander
Copy link
Contributor

@Rick-KLN, there's an quick fick for that. Edit http.js (done method) to check if digest is already running.

if(!$rootScope.$$phase) {
    $rootScope.$apply();
}

I forked it (https://github.com/The-Amazing-Society/angular.js/commit/623e12aea0a4fefa5d4225e72181b75bcec0dd5a) to implement this, had the same problem.

@dbinit
Copy link
Contributor Author

dbinit commented Jan 23, 2013

Thanks guys. It looks like I incorrectly assumed that abort would fire the onreadystatechange event separately. I'll push a fix + test in a little bit here.

@fredrikbonander
Copy link
Contributor

@dbinit Great work, I borrowed code from you fork. I would be real nice to get this merged in.

@rickkln
Copy link

rickkln commented Jan 23, 2013

Thanks a lot guys!

@TomKaltz
Copy link

Will this ever get merged into a production release?

@steinerj
Copy link

steinerj commented Mar 7, 2013

+1 on the merge :)

New $http specific method "abort" for promises. There are many cases
where a long-running request might need to be interrupted, e.g. a view
change.

Implementation details:
- after calling abort(), the promise is guaranteed to be resolved with a
  rejection (unless it has already been resolved)
- abort() returns true if successfully aborted (or already aborted), or
  false if the abort failed because the promise was already resolved
- has mock $httpBackend support

Closes angular#1159
@g00fy-
Copy link

g00fy- commented Apr 12, 2013

+1

@bracketdash
Copy link

+1 for merging this. Also looking forward to something similar for $resource.

@svileng
Copy link

svileng commented Apr 17, 2013

+1

@knalli
Copy link

knalli commented Apr 18, 2013

👍 (Probably needs to be merged again).

Okay, I was surprised at the lack of http's abort... so: What does this PR hold back? How we can help?

@damrbaby
Copy link
Contributor

How would you abort a pending request if you are using $resource?

@g00fy-
Copy link

g00fy- commented Apr 18, 2013

@damrbaby add a $abort method to the resource, that would fallback to $http's abort ?

@dbinit
Copy link
Contributor Author

dbinit commented Apr 18, 2013

Unfortunately I don't think this is going to be accepted as is. There is still ongoing discussion as to what the best approach is.

Using a $q progress callback might be more suitable (see #2223). If returning a rejection from a progress callback could abort the XHR, it would be almost equivalent to this.

@dbinit
Copy link
Contributor Author

dbinit commented Apr 19, 2013

I did a bit more research and found several implementations of Deferred/Promise that have an optional "canceler" feature.

I've submitted #2452 to implement that feature. If it's accepted, adding $http cancellation would be fairly trivial.

@dbinit dbinit closed this Apr 19, 2013
@aminariana
Copy link

+1,
need a cancel feature to avoid querying race condition.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.